Skip to content

SDKS-4738: Migrate Legacy SDK To OIDC Client In Reactjs Davinci Sample App#92

Merged
SteinGabriel merged 1 commit intomainfrom
SDKS-4738
Feb 27, 2026
Merged

SDKS-4738: Migrate Legacy SDK To OIDC Client In Reactjs Davinci Sample App#92
SteinGabriel merged 1 commit intomainfrom
SDKS-4738

Conversation

@SteinGabriel
Copy link
Contributor

@SteinGabriel SteinGabriel commented Feb 14, 2026 •
edited
Loading

https://pingidentity.atlassian.net/browse/SDKS-4738

Description

Migrates React DaVinci Todo sample app from legacy SDK (@forgerock/javascript-sdk) to OIDC Client (@forgerock/oidc-client), aligning with patterns established in React Journey sample app.

Changes

New Files:

  • client/context/oidc.context.js - OIDC context provider managing client initialization and auth state
  • client/context/protect.context.js - Protect API context provider (bootstrap mode)
  • client/context/theme.context.js - Theme context provider (migrated from global-state)

Deleted Files:

  • client/global-state.js - Removed AppContext in favor of dedicated context providers

Key Modifications:

  • client/index.js - Bootstrap pattern with Init component and loading gate (matches react-journey)
  • client/utilities/request.js - Replace HttpClient.request with native fetch + Bearer token
  • client/utilities/route.js - Validate routes using oidcClient.token.get() and oidcClient.user.info()
  • client/components/davinci-client/hooks/oauth.hook.js - Token exchange via oidcClient.token.exchange()
    (previously TokenManager.getTokens())
  • client/components/layout/header.js - Pass all query params to login link (not just centralLogin/journey)
  • All components - Migrated from AppContext to OidcContext, ProtectContext, and ThemeContext
  • package.json - Swap @forgerock/javascript-sdk → @forgerock/oidc-client
  • JSDoc comments - Updated to reflect OIDC client implementation; copyright year updated to 2026

@SteinGabriel SteinGabriel changed the title SDKS-4738: Migrate Legacy SDK To OIDC Client SDKS-4738: Migrate Legacy SDK To OIDC Client In Reactjs Davinci Sample App Feb 18, 2026
cerebrl
cerebrl requested changes Feb 19, 2026
View reviewed changes
Copy link
Contributor

@cerebrl cerebrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments after an initial review.

javascript/reactjs-todo-davinci/client/components/davinci-client/hooks/oauth.hook.js Outdated
forceRenew: true,
});
const tokens = await oidcState.oidcClient.token.exchange(authCode, authState);
if (tokens && typeof tokens === 'object' && 'error' in tokens) {
Copy link
Contributor

@cerebrl cerebrl Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being that these OIDC methods always return an object type, do we need all of these chained conditionals? From a type perspective, only OauthTokens | TokenExchangeErrorResponse | GenericError can be returned, so tokens should be truthy and always an object. The question should be limited to just "does this object have an error property", yeah? Or, am I missing something?

Copy link
Contributor Author

@SteinGabriel SteinGabriel Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I went extra defensive here since I wasn't really sure if tokens would always be returned as an object type. Thanks for clarifying this. I'll update it.

Copy link
Contributor Author

@SteinGabriel SteinGabriel Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

javascript/reactjs-todo-davinci/client/components/davinci-client/hooks/oauth.hook.js Outdated
const [user, setUser] = useState(null);
const [authCode, setAuthCode] = useState(null);
const [authState, setAuthState] = useState(null);
const [oidcState] = useContext(OidcContext);
Copy link
Contributor

@cerebrl cerebrl Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to have many different patterns with this oidc related context usage. We have the following:

  1. const [oidcState] = useContext(OidcContext);
  2. const [{ oidcClient }] = useContext(OidcContext);
  3. const [state] = useContext(OidcContext);

I'd like to drop #3 as a pattern and just use #1 for accessing the state props, like username, etc., and use #2 for when we just want the oidcClient.

Copy link
Contributor Author

@SteinGabriel SteinGabriel Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure. Updated, thanks!

ancheetah
ancheetah requested changes Feb 19, 2026
View reviewed changes
Copy link
Contributor

@ancheetah ancheetah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration looks good overall. Left some comments about removing try-catches and not throwing errors. Some other notes:

  • Please update theming in every component, not just some. For example, icons. You can use AI to help.
  • Please ensure CI passes. I can help teach you how to run e2e locally if you need help.

ancheetah
ancheetah reviewed Feb 24, 2026
View reviewed changes
ancheetah
ancheetah reviewed Feb 24, 2026
View reviewed changes
Copy link
Contributor

@ancheetah ancheetah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of Protect stuff

ancheetah
ancheetah reviewed Feb 25, 2026
View reviewed changes
ancheetah
ancheetah approved these changes Feb 26, 2026
View reviewed changes
Copy link
Contributor

@ancheetah ancheetah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job. Approving now but before merging let's squash everything into one commit and wait till Justin gets back in case he has last minute feedback.

cerebrl
cerebrl approved these changes Feb 27, 2026
View reviewed changes
Copy link
Contributor

@cerebrl cerebrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks, @SteinGabriel !!

@SteinGabriel
feat(reactjs-todo-davinci): migrate legacy SDK to OIDC client
518947f 
fix: pass whole query params to signin link in header.js

feat: replace HTTP client with plain fetch

feat: add oidcClient and protectApi to global state

docs: update README file

refactor(reactjs-todo-davinci): refactor state menagement

fix(reactjs-todo-davinci): update copyright date

fix(reactjs-todo-davinci): fix list issues; update comments; fix code issues;

fix(reactjs-todo-davinci): remove REALM_PATH constant

fix(reactjs-todo-davinci): fix inconsistencies with pretect initialization

fix(reactjs-todo-davinci): remove protect.api.js; update copyright message;

fix(reactjs-todo-davinci): remove protect.api.js; update copyright message;

fix(reactjs-todo-davinci): make initProtectApi more explicit; avoid throwing errors;

fix(reactjs-todo-davinci): remove create-config; remove try/catch;

fix(reactjs-todo-davinci): remove initProtectApi utility function

fix(reactjs-todo-davinci): remove protectApi check in protect.js
@SteinGabriel SteinGabriel merged commit e83ce26 into main Feb 27, 2026
16 checks passed
@SteinGabriel SteinGabriel deleted the SDKS-4738 branch February 27, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cerebrl cerebrl cerebrl approved these changes

@ancheetah ancheetah ancheetah approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SteinGabriel @cerebrl @ancheetah